Skip to content

Conversation

@drgroot
Copy link
Member

@drgroot drgroot commented Aug 12, 2025

No description provided.

Copilot AI review requested due to automatic review settings August 12, 2025 14:53
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds the ability to use a custom prefix for Delta table locations by introducing a new delta-prefix configuration option that takes precedence over the existing s3.access-key-id fallback.

  • Introduces delta-prefix configuration option for customizing table location prefixes
  • Maintains backward compatibility by falling back to s3.access-key-id when delta-prefix is not specified

self._location_prefix = os.path.join(
str(catalog_properties_raw.get("warehouse")),
str(catalog_properties_raw.get("s3.access-key-id")),
str(catalog_properties_raw.get("delta-prefix") or catalog_properties_raw.get("s3.access-key-id")),
Copy link

Copilot AI Aug 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using an AWS access key ID as a fallback for a table location prefix is questionable from a security perspective, as it exposes sensitive credentials in file paths. Consider using a dedicated configuration option or a more appropriate default value.

Suggested change
str(catalog_properties_raw.get("delta-prefix") or catalog_properties_raw.get("s3.access-key-id")),
str(catalog_properties_raw.get("delta-prefix") or "default-delta-prefix"),

Copilot uses AI. Check for mistakes.
@drgroot drgroot merged commit f97d751 into main Aug 12, 2025
25 checks passed
@drgroot drgroot deleted the fix/prefix branch August 12, 2025 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants